v.in.dwg: Avoid using same variable as parameter and destination in sprintf #4262
Conversation
github-actions
bot
added
vector
|
Note: I couldn't compile the files in this package as I was not able to download the OpenDWG toolkit headers and libraries from http://www.opendwg.org from instructions in README. I believe the link is dead. So, I'm a bit uneasy about these changes. But, it's still a problem at hand as current code can cause undefined behavior. I want to get inputs from you all on this. |
ymdatta
changed the title
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
ymdatta
force-pushed
the
cppcheck_sprintfoverlappingdata
branch
from
03093da to
962be87
Compare
Currently, one instance of sprintf has same variable as parameter and destination in sprintf. This scneario leads to undefined behavior in C. Modify the code to: 1. Write initial error string using snprintf() onto the buffer. Using snprintf() makes sure that we stay within the buffer size and avoid overflow errors. 2. Use snprintf() again to write another error string at the end of previous error string in the same buffer. We again use snprintf() to make sure we are not overflowing the buffer with data. Signed-off-by: Mohan Yelugoti <ymdatta.work@gmail.com>
ymdatta
force-pushed
the
cppcheck_sprintfoverlappingdata
branch
from
962be87 to
fc5982c
Compare
ymdatta
changed the title
I have search around and the OpenDWG page went offline around 2011. Perhaps we should rather retire this code? |
I think we should. We have v.in.redwg as replacement. |
|
That makes sense. By retiring the code, do we stop pushing new commits to this module? If so, I can close this PR. |
|
@nilason if your comment was resolved, I suggest merging and then removing the disabled code in another PR. If it is not resolved, I suggest closing and, again, removing. |
@ymdatta did resolve the changes of the code and came up a nice string catenation solution using snprinft() (strlen() was the key). |
|
The update is appreciated and then we may move it to add-ons, in a good shape. |
Move is something else than remove :-) In not sure about the good shape, this has probably not been tested (or even used) in quite a while (depending on proprietary SDK), although it will certainly be better! |
Just because @ymdatta already created the changes. I would not be suggesting that otherwise. I'm saying, let's merge this (rather than close), and then remove the code (directory and Makefile entry). |
|
@ymdatta, can you please create a PR to remove v.in.dwg? |
Currently, one instance of sprintf has same variable as parameter and destination in sprintf. This scneario leads to undefined behavior in C.
Modify the code to:
This was found using cppcheck tool.
Prior to fix:
After fix:
Additional information: